-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used #40889
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| node | ||
| } else { | ||
| val newNode = addMetadataCol(node) | ||
| val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this only does addMetadataCol on children?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is from this fix #39895
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks correct to me because we need to propagate metadata cols to this node and need to add metadata cols in its children. Does master branch have the same code here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, master branch has the same code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, there was follow up.
RussellSpitzer
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! If anyone else was seeing odd issues with mispropagated metadata columns this should fix for them as well.
### What changes were proposed in this pull request? backporting #39152 to 3.3 ### Why are the changes needed? bug fixing ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? UT Closes #40889 from huaxingao/metadata. Authored-by: huaxingao <[email protected]> Signed-off-by: huaxingao <[email protected]>
|
Merged to branch-3.3. Thank you all for reviewing! |
What changes were proposed in this pull request?
backporting #39152 to 3.3
Why are the changes needed?
bug fixing
Does this PR introduce any user-facing change?
No
How was this patch tested?
UT